[#964] Add IPFS response size limit#972
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
project7-interns
left a comment
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The new Content-Length guard is good, but the fallback body-size check is measuring JavaScript string length rather than response bytes. That means a UTF-8 response with multi-byte characters can exceed the intended 1MB limit and still pass.
Findings
- [medium] The post-read size validation is not actually checking bytes.
ipfsContent.length/text.lengthcounts UTF-16 code units, not the number of bytes received over the network. A malicious IPFS response containing multi-byte UTF-8 characters can be larger than 1,000,000 bytes while still staying under the character-count threshold, so the "actual body size" defense-in-depth check is incomplete.- File:
src/app/api/index/storyline/route.ts:126,src/app/api/index/plot/route.ts:98,src/app/api/cron/backfill/route.ts:42 - Suggestion: Measure bytes instead, for example with
new TextEncoder().encode(text).lengthor by enforcing the limit while streaming the response body.
- File:
Decision
Requesting changes because the current implementation can still allow oversized IPFS bodies through when the response uses multi-byte characters.
…ing length Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The re-review update fixes the body-size validation by measuring UTF-8 byte length with TextEncoder rather than string length, so the 1MB cap now applies correctly to multi-byte IPFS responses as well. The header check plus byte-count fallback are in place for storyline, plot, and backfill paths.
Findings
- No blocking findings.
Decision
Approving because the previous correctness issue in the IPFS size-limit enforcement is resolved and the PR now satisfies the reviewed acceptance criteria. Checks visible to me were still pending at review time.
2 participants
Summary
Content-Lengthheader (fast reject) and actual body length (defense in depth)Fixes #964
Test plan
🤖 Generated with Claude Code